Revert "fix: Adding lock to init relayer instances"#692
Merged
collins-w merged 1 commit intoMar 13, 2026
Conversation
This reverts commit 51df3c5.
WalkthroughThis PR removes distributed locking and coordination infrastructure from the bootstrap and relayer initialization processes. It eliminates lock-based sequential flows, polling utilities, and Redis metadata tracking helpers, replacing them with simplified decision logic and concurrent initialization patterns. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.16.4)src/bootstrap/initialize_relayers.rs┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m src/bootstrap/config_processor.rs┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m src/repositories/api_key/api_key_redis.rs┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m Comment Tip CodeRabbit can suggest fixes for GitHub Check annotations.Configure the |
tirumerla
approved these changes
Mar 13, 2026
Codecov Report❌ Patch coverage is
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. @@ Coverage Diff @@
## main #692 +/- ##
==========================================
+ Coverage 90.27% 91.01% +0.73%
==========================================
Files 289 289
Lines 120108 118712 -1396
==========================================
- Hits 108428 108043 -385
+ Misses 11680 10669 -1011
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bootstrap/config_processor.rs (1)
353-384:⚠️ Potential issue | 🔴 Critical
is_redis_populated()is not a safe bootstrap gate.This check is neither complete nor atomic. A previous failed bootstrap that wrote only some repositories will now cause later startups to skip the missing ones forever, and
api_key_repositoryis not part of the populated check, so an API-key-only Redis still falls through toprocess_api_key()and hits a duplicatedefault. On top of that, two nodes sharing Redis can both observe "empty" and race through the create/drop flow. Please restore an explicit completion marker plus distributed serialization here, or make the whole bootstrap idempotent end-to-end.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bootstrap/config_processor.rs` around lines 353 - 384, The bootstrap gate relying on is_redis_populated() is unsafe; replace it with a distributed lock + explicit completion marker in Redis to serialize and mark successful bootstraps: acquire a Redis-based lock (e.g., SETNX + TTL) before performing any create/drop logic (referencing is_redis_populated, should_process_config_file, reset_storage_on_start, and app_state), skip processing only if a completion marker key exists, and after all process_* calls (process_plugins, process_signers, process_notifications, process_networks, process_relayers, process_api_key) set the completion marker atomically; also include api_key_repository in the population/marker logic or make process_api_key idempotent to avoid duplicate default key creation and ensure safe retries on failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bootstrap/initialize_relayers.rs`:
- Around line 85-95: The current use of try_join_all over relayers only
parallelizes initialize_relayer within this process but removes the prior
cross-process distributed init guard, causing duplicate side effects across
nodes; restore cross-process coordination by acquiring the repository-wide/init
guard (or a persistent per-relayer lock/flag) before calling initialize_relayer
for each relayer and skip initialization on nodes that do not hold the
guard—wrap the logic that currently constructs relayer_futures/awaits
try_join_all with a check/acquire of the distributed guard (or per-relayer
guard) so only the holder runs initialize_relayer(relayer.id.clone(), app_state)
while others skip or await completion.
In `@src/repositories/api_key/api_key_redis.rs`:
- Around line 333-340: Logging in RedisApiKeyRepository incorrectly refers to
"plugin" instead of "API key"; update log messages and any comments near the
shown block (and similarly in the subsequent blocks around lines 350-380) to use
API-key terminology. Specifically, change the debug strings around the check
using plugin_list_key (the conn.exists call and its surrounding debug!("Checking
if plugin entries exist") / debug!("Plugin entries exist: {}")) to something
like "Checking if API key entries exist" and "API key entries exist: {}", and
apply the same rename to any deletes/reads in the methods that call
self.map_redis_error(...) (preserve the existing error mapping call and variable
names like plugin_list_key but ensure logs/comments say API key).
---
Outside diff comments:
In `@src/bootstrap/config_processor.rs`:
- Around line 353-384: The bootstrap gate relying on is_redis_populated() is
unsafe; replace it with a distributed lock + explicit completion marker in Redis
to serialize and mark successful bootstraps: acquire a Redis-based lock (e.g.,
SETNX + TTL) before performing any create/drop logic (referencing
is_redis_populated, should_process_config_file, reset_storage_on_start, and
app_state), skip processing only if a completion marker key exists, and after
all process_* calls (process_plugins, process_signers, process_notifications,
process_networks, process_relayers, process_api_key) set the completion marker
atomically; also include api_key_repository in the population/marker logic or
make process_api_key idempotent to avoid duplicate default key creation and
ensure safe retries on failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ab8d2409-c3d1-4f05-b164-6e6e55aac141
📒 Files selected for processing (9)
src/bootstrap/config_processor.rssrc/bootstrap/initialize_relayers.rssrc/repositories/api_key/api_key_redis.rssrc/repositories/relayer/mod.rssrc/repositories/relayer/relayer_in_memory.rssrc/repositories/relayer/relayer_redis.rssrc/utils/mod.rssrc/utils/polling.rssrc/utils/redis.rs
💤 Files with no reviewable changes (6)
- src/repositories/relayer/relayer_redis.rs
- src/repositories/relayer/mod.rs
- src/repositories/relayer/relayer_in_memory.rs
- src/utils/mod.rs
- src/utils/polling.rs
- src/utils/redis.rs
Comment on lines
+85
to
95
| debug!(count = relayers.len(), "Initializing relayers concurrently"); | ||
|
|
||
| if completed { | ||
| info!("Initialization completed by another instance during extended wait"); | ||
| Ok(()) | ||
| } else { | ||
| // Extended wait also timed out (~260s total). Proceed with initialization | ||
| // as a last resort rather than failing — duplicate side effects (notifications, | ||
| // jobs) are a minor cost compared to no instance initializing at all. | ||
| warn!("Extended wait also timed out, proceeding with initialization"); | ||
| let result = run_initialization_batch(relayers, app_state).await; | ||
| if result.is_ok() { | ||
| if let Err(e) = set_global_init_completed(conn, prefix).await { | ||
| warn!(error = %e, "Failed to record initialization completion time"); | ||
| } | ||
| } | ||
| result | ||
| } | ||
| } | ||
| Err(e) => { | ||
| // Redis error — graceful degradation | ||
| warn!( | ||
| error = %e, | ||
| "Failed to check lock after timeout, attempting initialization without coordination" | ||
| ); | ||
| run_initialization_batch(relayers, app_state).await | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Runs the batch initialization of all relayers concurrently. | ||
| async fn run_initialization_batch<J, RR, TR, NR, NFR, SR, TCR, PR, AKR>( | ||
| relayers: &[RelayerRepoModel], | ||
| app_state: &ThinDataAppState<J, RR, TR, NR, NFR, SR, TCR, PR, AKR>, | ||
| ) -> Result<()> | ||
| where | ||
| J: JobProducerTrait + Send + Sync + 'static, | ||
| RR: RelayerRepository + Repository<RelayerRepoModel, String> + Send + Sync + 'static, | ||
| TR: TransactionRepository + Repository<TransactionRepoModel, String> + Send + Sync + 'static, | ||
| NR: NetworkRepository + Repository<NetworkRepoModel, String> + Send + Sync + 'static, | ||
| NFR: Repository<NotificationRepoModel, String> + Send + Sync + 'static, | ||
| SR: Repository<SignerRepoModel, String> + Send + Sync + 'static, | ||
| TCR: TransactionCounterTrait + Send + Sync + 'static, | ||
| PR: PluginRepositoryTrait + Send + Sync + 'static, | ||
| AKR: ApiKeyRepositoryTrait + Send + Sync + 'static, | ||
| { | ||
| let futures = relayers.iter().map(|relayer| { | ||
| let relayer_futures = relayers.iter().map(|relayer| { | ||
| let app_state = app_state.clone(); | ||
| let relayer_id = relayer.id.clone(); | ||
|
|
||
| async move { | ||
| let result = initialize_relayer(relayer_id.clone(), app_state).await; | ||
| (relayer_id, result) | ||
| } | ||
| async move { initialize_relayer(relayer.id.clone(), app_state).await } | ||
| }); | ||
|
|
||
| let results = futures::future::join_all(futures).await; | ||
|
|
||
| // Count and report results | ||
| let succeeded = results.iter().filter(|(_, r)| r.is_ok()).count(); | ||
| let failed = results.iter().filter(|(_, r)| r.is_err()).count(); | ||
| try_join_all(relayer_futures) | ||
| .await | ||
| .wrap_err("Failed to initialize relayers")?; | ||
|
|
There was a problem hiding this comment.
try_join_all doesn't replace the distributed init guard.
This only makes relayer initialization concurrent within one process. After removing cross-process coordination, every node sharing the repository will still initialize the same relayers at startup. The downstream init path mutates relayer state and can enqueue notifications and health-check jobs, so duplicate startups now duplicate those side effects.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bootstrap/initialize_relayers.rs` around lines 85 - 95, The current use
of try_join_all over relayers only parallelizes initialize_relayer within this
process but removes the prior cross-process distributed init guard, causing
duplicate side effects across nodes; restore cross-process coordination by
acquiring the repository-wide/init guard (or a persistent per-relayer lock/flag)
before calling initialize_relayer for each relayer and skip initialization on
nodes that do not hold the guard—wrap the logic that currently constructs
relayer_futures/awaits try_join_all with a check/acquire of the distributed
guard (or per-relayer guard) so only the holder runs
initialize_relayer(relayer.id.clone(), app_state) while others skip or await
completion.
Comment on lines
+333
to
+340
| debug!("Checking if plugin entries exist"); | ||
|
|
||
| let exists: bool = conn | ||
| .exists(&plugin_list_key) | ||
| .await | ||
| .map_err(|e| self.map_redis_error(e, "has_entries_check"))?; | ||
|
|
||
| debug!("API key entries exist: {}", exists); | ||
| debug!("Plugin entries exist: {}", exists); |
There was a problem hiding this comment.
Restore API-key terminology in this repository's logging.
These branches still read and delete API key keys, but the new messages/comments now say "plugin". That will mislead storage-reset and bootstrap debugging inside RedisApiKeyRepository.
Also applies to: 350-380
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/repositories/api_key/api_key_redis.rs` around lines 333 - 340, Logging in
RedisApiKeyRepository incorrectly refers to "plugin" instead of "API key";
update log messages and any comments near the shown block (and similarly in the
subsequent blocks around lines 350-380) to use API-key terminology.
Specifically, change the debug strings around the check using plugin_list_key
(the conn.exists call and its surrounding debug!("Checking if plugin entries
exist") / debug!("Plugin entries exist: {}")) to something like "Checking if API
key entries exist" and "API key entries exist: {}", and apply the same rename to
any deletes/reads in the methods that call self.map_redis_error(...) (preserve
the existing error mapping call and variable names like plugin_list_key but
ensure logs/comments say API key).
collins-w
deleted the
revert-622-improvements-to-relayers-initialization
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reverts #622
Summary by CodeRabbit